WESTMIDLANDS | ITP-MAY-25 | AHMAD EHSAS | MODULE STRUCTURING AND TESTING DATA SPRINT-3 #656
Conversation
…, and invalid cards
…he goal of this function is to count how many times a specific character appears in a given string
cjyuan
added
the
Review in progress
|
|
||
| return "Invalid angle"; |
There was a problem hiding this comment.
Technically angles less than or equal to 0 are also considered "Invalid angle".
| if (Math.abs(numerator) >= Math.abs(denominator)) return false; | ||
| if (Math.abs(numerator) === Math.abs(denominator)) return false; | ||
| } |
There was a problem hiding this comment.
Can you use else to simplify your code?
| if (rank === "A") return 11; | ||
| if (["K", "Q", "J"].includes(rank)) return 10; | ||
|
|
||
| const number = parseInt(rank); |
There was a problem hiding this comment.
Does your function return the value you expected from each of the following function calls?
getCardValue("0x02♠");
getCardValue("2.1♠");
getCardValue("0002♠");
getCardValue("3ABC♠");
Consider looking up how parseInt() work.
| test("should return 5 for 5 of Hearts", () => { | ||
| const fiveofHearts = getCardValue("5♥"); | ||
| expect(fiveofHearts).toEqual(5); | ||
| }); | ||
|
|
||
| test("should return 10 for 10 of Diamonds", () => { | ||
| const tenofDiamonds = getCardValue("10♦"); | ||
| expect(tenofDiamonds).toEqual(10); | ||
| }); |
There was a problem hiding this comment.
When preparing tests, we should ensure the tests cover all possible cases. If we specify a test for individual card, we will need about 53 tests to cover all possible cases. Instead, we could consider classifying all possible values into different categories, and then within each category we test some samples.
For example, one possible category for getCardValue() is, "should return the value of number cards (2-10)", and we can prepare the test as
test("should return the value of number cards (2-10)", () => {
expect(getCardValue("2♣︎")).toEqual(2);
expect(getCardValue("5♠")).toEqual(5);
expect(getCardValue("10♥")).toEqual(10);
// Note: We could also use a loop to check all values from 2 to 10.
});
| test("should return 10 for Jack of Clubs", () => { | ||
| const jackofClubs = getCardValue("J♣"); | ||
| expect(jackofClubs).toEqual(10); | ||
| }); | ||
|
|
||
| test("should return 10 for Queen of Hearts", () => { | ||
| const queenofHearts = getCardValue("Q♥"); | ||
| expect(queenofHearts).toEqual(10); | ||
| }); | ||
|
|
||
| test("should return 10 for King of Spades", () => { | ||
| const kingofSpades = getCardValue("K♠"); | ||
| expect(kingofSpades).toEqual(10); | ||
| }); |
There was a problem hiding this comment.
We could combine these tests and place them under the category "should return 10 for face cards (J, Q, K)".
| try { | ||
| getCardValue("Z♠"); | ||
| } catch (error) { | ||
| console.log("Caught error:", error.message); | ||
| } |
There was a problem hiding this comment.
Jest offers a method called toThrow() that can be used to check if a function can throw the expected error.
You can find out more about how to use .toThrow() here: https://jestjs.io/docs/expect#tothrowerror (Note: Pay close attention to the syntax of the example)
| if (num === 1) { | ||
| return "1st"; | ||
| } |
There was a problem hiding this comment.
This function is not fully implemented; it should work for any valid positive integers.
Consider looking up the rules to clarify how ordinal numbers are formed.
There was a problem hiding this comment.
The tests in this Jest script are incomplete. When you have fully implemented the function in Sprint-3/3-mandatory-practice/implement/get-ordinal-number.js, can you prepare tests in this script to cover all valid integers in the following way:
Group all possible input values into meaningful categories, and then, select representative samples from each category to test. This approach improves coverage and makes our tests easier to maintain.
For example, we can prepare a test for numbers 2, 22, 132, etc. as
test("append 'nd' to numbers ending in 2, except those ending in 12", () => {
expect( getOrdinalNumber(2) ).toEqual("2nd");
expect( getOrdinalNumber(22) ).toEqual("22nd");
expect( getOrdinalNumber(132) ).toEqual("132nd");
});
There was a problem hiding this comment.
Why not update the tests in Sprint-3/3-mandatory-practice/implement/repeat.test.js to check your implementation?
cjyuan
added
Reviewed
Self checklist
Changelist
Briefly explain your PR.
Completed sprint-3, dealing with errors and understanding the flow of a program
Questions